Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ADAPT-000: Remove link-underline utility class #851

Merged
merged 9 commits into from
Sep 17, 2021
Merged

ADAPT-000: Remove link-underline utility class #851

merged 9 commits into from
Sep 17, 2021

Conversation

sherakama
Copy link
Member

@sherakama sherakama commented Sep 17, 2021 •
edited
Loading

READY FOR REVIEW

Summary

  • Removes link-underline utility classes as they don't work for JIT mode
  • Updated to node version 16 and updated a few packages

Steps to Test

  1. Review code changes
  2. Review preview build
  3. Review upgrade.md dox

Affected Projects or Products

  • Decanter React
  • Alumni

Associated Issues and/or People

  • JIRA ticket
  • Other PRs
  • Any other contextual information that might be helpful (e.g., description of a bug that this PR fixes, new functionality that it adds, etc.)
  • Anyone who should be notified? (@mention them here)

See Also

yvonnetangsu
yvonnetangsu reviewed Sep 17, 2021
View reviewed changes
package.json Outdated Show resolved Hide resolved
yvonnetangsu
yvonnetangsu reviewed Sep 17, 2021
View reviewed changes
Copy link
Member

@yvonnetangsu yvonnetangsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just that one question - the rest looks good to me!

@github-actions github-actions bot added size/s and removed size/xs labels Sep 17, 2021
@sherakama sherakama closed this Sep 17, 2021
@sherakama
Copy link
Member Author

Aborting as there is no good way to make this plugin work with JIT mode. We are going to have to remove it.

@yvonnetangsu
Copy link
Member

Aborting as there is no good way to make this plugin work with JIT mode. We are going to have to remove it.

Aww...bummer. Thank you for testing though! We'll find ways to work around that.

@sherakama sherakama reopened this Sep 17, 2021
@github-actions github-actions bot added size/l and removed size/s labels Sep 17, 2021
@sherakama sherakama added the beta Releases a beta tag label Sep 17, 2021
@sherakama sherakama changed the title ADAPT-000: Fix CSS in JS Syntax. ADAPT-000: Remove link-underline utility class Sep 17, 2021
@sherakama
Copy link
Member Author

@rebeccahongsf @yvonnetangsu re-ready for review.

yvonnetangsu
yvonnetangsu approved these changes Sep 17, 2021
View reviewed changes
Copy link
Member

@yvonnetangsu yvonnetangsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for investigating, @sherakama . GTG - bummer this doesn't work with JIT.
I suspect we also need to remove the link-color and link-fontweight utilities, but we could address that in another PR perhaps?
https://github.com/SU-SWS/decanter/blob/v7/src/plugins/utilities/link/link.js
https://github.com/SU-SWS/decanter/blob/v7/src/plugins/utilities/link/link-fontweight.js

@sherakama
Copy link
Member Author

Font weight should be fine but the hovers in link will have to go...

@github-actions github-actions bot added size/xl and removed size/l labels Sep 17, 2021
@sherakama
Copy link
Member Author

sherakama commented Sep 17, 2021 •
edited
Loading

Thanks for investigating, @sherakama . GTG - bummer this doesn't work with JIT.
I suspect we also need to remove the link-color and link-fontweight utilities, but we could address that in another PR perhaps?
v7/src/plugins/utilities/link/link.js
v7/src/plugins/utilities/link/link-fontweight.js

@yvonnetangsu,

Good catch. The font-weight should be fine as it doesn't have the hacky variant classes (hover, focus) but the others have had their hacks removed. I restored the main selector for now but these should really be considered deprecated at this point. We will remove them from our other work first before completely removing them from this package.

@sherakama sherakama mentioned this pull request Sep 17, 2021
Merged
@yvonnetangsu
Copy link
Member

@sherakama This looks good to me. Thank you for the fix!

@yvonnetangsu yvonnetangsu merged commit 0e2ffca into v7 Sep 17, 2021
@yvonnetangsu yvonnetangsu deleted the task/underline branch September 17, 2021 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yvonnetangsu yvonnetangsu yvonnetangsu approved these changes

@rebeccahongsf rebeccahongsf Awaiting requested review from rebeccahongsf

Assignees
No one assigned
Labels
beta Releases a beta tag size/xl
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sherakama @yvonnetangsu